Skip to content
This repository has been archived by the owner on Jan 6, 2023. It is now read-only.

fix(resources): Always use / in paths written to config.xml #69

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jakub-g
Copy link

@jakub-g jakub-g commented Feb 12, 2016

fixes ionic-team/ionic-cli#625
fixes ionic-team/ionic-framework#4212

Let's update image path just before writing to config.xml so that we have always / regardless of platform.

@jakub-g
Copy link
Author

jakub-g commented Feb 13, 2016

So to start the discussion, this bug happens because path.join() uses \ on Windows while / on other platforms. We could argue where is the best place to normalize paths again to use /, I think just before saving the config file is a good choice.

Let me know what you think about this patch; do you want a comment in code? extract it as a separate method maybe?

This is a small patch and a number of people are waiting for it, so I'd love to have it merged soon :)

Thanks
Jakub

@tlancina
Copy link
Contributor

Thanks for the PR!

I like doing it before saving in config.xml, because then two people running on different platforms won't constantly be changing it back and forth in version control.

Will see to getting this merged in soon, please ping me in a few days if it has not been closed.

@klh
Copy link

klh commented May 27, 2016

change the dependency on Path to Upath instead, which does platform detection before concatenating paths. a safer code change. https://www.npmjs.com/package/upath

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants